-
Notifications
You must be signed in to change notification settings - Fork 17
IBX-3035: [Twig] Allowed passing parameters to ibexa render function
#674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.6
Are you sure you want to change the base?
IBX-3035: [Twig] Allowed passing parameters to ibexa render function
#674
Conversation
…ibexa_render Twig function
2e09071 to
1bb7be8
Compare
1bb7be8 to
2a1ab1c
Compare
|
| ); | ||
| } | ||
|
|
||
| public static function forwardOptionsToFragmentRenderer(MockObject $fragmentRendererMock, MockObject $valueObjectMock, string $renderStrategyClass): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This abstractions is made to avoid cut&paste of code for similar test in RenderLocationStrategyTest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This abstractions is made to avoid cut&paste of code for similar test in RenderLocationStrategyTest
But does it have to be static?
| if ($request->attributes->has('viewParametersString')) { | ||
| $uri->attributes['viewParametersString'] = $request->attributes->get('viewParametersString'); | ||
| } | ||
| if ($options['params'] ?? false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refactor this so you don't rely on true-ish expression inside of the if statement. This works because of implicit cast which can lead to some bugs under some circumstances. What is the real condition making this usable? !is_empty && is_array?
| } | ||
| if ($options['params'] ?? false) { | ||
| $uri->attributes['params'] = $options['params']; | ||
| unset($options['params']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effectively changes the behavior which we can do in 6.0 ATM.
| $renderer = $this->getFragmentRenderer($options->get('method', $this->defaultRenderer)); | ||
|
|
||
| return $renderer->render($controllerReference, $currentRequest)->getContent(); | ||
| return $renderer->render($controllerReference, $currentRequest, $options->has('params') ? ['params' => $options->get('params')] : [])->getContent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readability: please split this into multiple lines.
Personally I'd just go with:
| return $renderer->render($controllerReference, $currentRequest, $options->has('params') ? ['params' => $options->get('params')] : [])->getContent(); | |
| return $renderer->render($controllerReference, $currentRequest, ['params' => $options->get('params', [])])->getContent(); |
and would've taken this into the account when processing further, effectively making params always present, reducing cognitive complexity.
But no strong feelings here.
| ); | ||
| } | ||
|
|
||
| public static function forwardOptionsToFragmentRenderer(MockObject $fragmentRendererMock, MockObject $valueObjectMock, string $renderStrategyClass): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This abstractions is made to avoid cut&paste of code for similar test in RenderLocationStrategyTest
But does it have to be static?
| ); | ||
| } | ||
|
|
||
| public static function forwardOptionsToFragmentRenderer(MockObject $fragmentRendererMock, MockObject $valueObjectMock, string $renderStrategyClass): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing PHPDoc defining type intersection for MockObject arguments.
ibexa render function



Description:
When using
render(controller('ibexa_content::viewAction')), you can send parameters to the controller which will be available in the template.Example:
This PR adds the same possibility when using
ibexa_render():For QA:
Documentation: